-
Notifications
You must be signed in to change notification settings - Fork 10.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
regression: Messagebox sending message instead of just selecting popup suggestion #32890
Conversation
Looks like this PR is ready to merge! 🎉 |
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #32890 +/- ##
===========================================
+ Coverage 55.49% 55.62% +0.12%
===========================================
Files 2637 2637
Lines 57393 57393
Branches 11892 11892
===========================================
+ Hits 31852 31922 +70
+ Misses 22847 22771 -76
- Partials 2694 2700 +6
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
||
await expect(poHomeChannel.composer).toHaveValue('/gimme '); | ||
|
||
await poHomeChannel.composer.fill(''); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we could move this to an afterEach? Since it could be helpful for the other tests as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If having the input empty is a prerequisite, it should be in the beforeEach
. and one after all to cleanup
also is not type
deprecated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this case I decided to use test
and test.step
, which does not provider before/after callbacks. This decision was made to favor performance of the tests, since have one test for each of those would open a new page during execution.
…ove/threadMetrics * 'develop' of github.com:RocketChat/Rocket.Chat: (26 commits) chore: Bump rocket.chat to 6.12.0-develop (#32936) test: Move Jest configuration to a package of presets (#32802) chore: bump turbo (#32938) feat: New users page deactivated tab and active tab ui (#32032) chore: bump traefik (#32892) test: fix flaky test `Archive department` (#32933) fix(Livechat): `After Registration Triggers` showing in wrong screen (#32928) refactor: Remove deprecated `Options.AvatarSize` constant (#32909) chore: improve `on login` cached collection (#32929) i18n: Rocket.Chat language update from Lingohub 🤖 on 2024-07-25Z (#32908) refactor: Circular imports (#32885) regression: notify user properly on logout (#32920) chore(client): stop replacing `Meteor.user` (#32910) regression: Messagebox sending message instead of just selecting popup suggestion (#32890) refactor: move broadcastMessageFromData to notifyListener (#32843) chore: prevent destructuring _id of deleted users (#32899) ci: increase kernel limits (#32902) ci: lint issues Release 6.10.1 fix: imported fixes (#32894) ...
Proposed changes (including videos or screenshots)
The way events were being attached changed and affected the popup and messagebox behaviour. When tagging or selecting another suggestion, it's currently filling the messagebox with the suggestion and sending the message at the same time.
This PR fixes this and adds tests to ensure this behaviour doesn't happen again.
Issue(s)
Introduced here: #32861
CORE-577
Steps to test or reproduce
Further comments